common: restore OpenMP thread limit after GraphicsMagick init - #22438

Claude Code says GM can't overrun its buffers when darktable raises the count. In the future, G'MIC 3.x and 4.x. could cause problems -- see end of report.

Also, 3 new (unrelated) bugs were found in XTrans-related code. I'll file those separately.

GM's init (magick/resource.c:676-689) does two things with threads. It calls omp_set_num_threads() on the calling thread, using OMP_NUM_THREADS or the CPU count, and it stores that number as its ThreadsResource limit. It allocates nothing
per-thread there.

Every per-thread array in GM is sized at the moment it is allocated, from the calling thread's omp_get_max_threads():

• Per-image pixel views: created by AllocateImage (image.c:380, sized at pixel_cache.c:517) and indexed by omp_get_thread_num() (pixel_cache.c:566).
• Per-operation scratch: omp_data_view.c:93, plus the pixel iterators and the gradient and drawing code.
• Thread-specific data: the variant sized by OpenMP is only compiled when neither pthreads nor Win32 is available (tsd.c:18-27), so it doesn't apply on our platforms.

In Pascal's example (GM picks 2, darktable restores 16), the restore runs inside dt_init(), before any GM image exists. Every image GM creates afterwards is sized for 16, and its parallel loops run on the same thread with 16 threads.

The stored "2" is only read in two places:

• the JPEG XL coder (coders/jxl.c:472), which passes it to libjxl's own thread pool; that pool isn't indexed by OpenMP thread number.
• the gm command-line tool, which darktable doesn't use.

GM does have one real constraint: an image must not be processed by more threads than existed when it was created. darktable creates, reads and destroys each GM image inside one function, with no count change in between.

On "fragile"

The earlier reply said "Codex read the GM sources", which invites the "we depend on GM internals" objection. The stronger argument doesn't need a GM audit:

• darktable already relied on this. Worker threads set their own count with omp_set_num_threads(dt_get_num_threads()) (src/control/jobs.c:509, :554). The OpenMP thread count is per thread, so GM's init never touched those threads. In the GUI, every GM load has always run with darktable's count, not GM's. That is exactly Pascal's scenario (OMP_NUM_THREADS=2, -t 16). The PR only makes the main thread, which darktable-cli renders on, behave like the workers.
• GM changes the count after init itself. SetMagickResourceLimit(ThreadsResource, n) calls omp_set_num_threads() (resource.c:1037), and gm benchmark uses it between iterations (command.c:2043). So GM can't assume the count it set at init stays.
• A second init doesn't undo the reset. libgmic is loaded with the lut3d plugin after the reset. If it calls InitializeMagick() again, GM returns immediately because it is already initialized (magick.c:1202).

Separate problem found while checking (not reproduced)

G'MIC 3.x and 4.x call omp_set_num_threads(nb_cpus) every time a gmic instance is constructed, unless OMP_NUM_THREADS is set (gmic-v.4.0.5/src/gmic.cpp:3603, reached from :4183). darktable constructs one in lut3d for compressed LUTs (src/iop/lut3dgmic.cpp:48, :105). With -t below the CPU count, that would reset the count on the calling thread (a pipe worker, or darktable-cli's main thread) in the middle of a session. The reset in dt_init() can't cover that. The installed G'MIC is 2.9.4, which doesn't do this, so I couldn't reproduce it. This is the genuinely fragile part, and adding num_threads(dt_get_num_threads()) to DT_OMP_FOR would fix it. It belongs in its own issue, not this PR.

Also: once #22456 and #22457 are fixed, a next fix can be added to allocate buffers based on the thread count, until that changing the allocation would change output. Over to Claude:

When OMP_THREAD_LIMIT is below -t, darktable still sizes its per-thread buffers for -t threads, although at most OMP_THREAD_LIMIT workers ever run. That wastes some memory but cannot overflow. Capping darktable.num_openmp_threads at omp_get_thread_limit() would avoid the waste, but the same count also decides how some modules split their work. X-Trans raw denoise builds its color planes in dt_get_num_threads() row chunks, and the chunk boundaries are where #22456 and #22457 change the output. With -t 4 and OMP_THREAD_LIMIT=1, the cap changed 35 pixels of the 0068 export (by up to 4/255). The integration runner (-t 4, OMP_THREAD_LIMIT=4) is not affected. So I'm keeping that change out of this PR. Once raw denoise no longer depends on the thread count, it can go in a separate PR, after checking other modules that split work by dt_get_num_threads().

@kofa73 : So if I understand correctly this works with current GM but will fail with GM > 3.x. That's not a nice future :)

What would be your proposal on this?

If we still want to merge this PR I would like to get more comments about the future GM 3.x and above issue that we are going to hit.

According to Claude Code's analysis:

This PR neither causes nor fixes it. The PR's reset runs once, in dt_init(). G'MIC changes the count later, whenever lut3d creates a gmic instance. The PR doesn't touch lut3d or G'MIC.

The risk applies to builds linked against a recent G'MIC:

• cmake/modules/FindGMIC.cmake:10,35 accepts G'MIC versions from 2.7.0 up to (but not including) 10.0, so 3.x and 4.x builds are supported.
• In 3.7.6 and 4.0.5, every gmic constructor reaches set_variable("_cpus", ...) (gmic.cpp:4183). That calls omp_set_num_threads(nb_cpus) when OMP_NUM_THREADS is unset (:3603). Claude Code hasn't checked when that behavior was introduced.

The path in darktable: commit_params() (src/iop/lut3d.c:1480) → _calculate_clut() → for a compressed .gmz LUT, lut3dgmic.cpp:48 or :105 → the gmic constructor. This runs on the pipe thread, so it affects the GUI too, not just darktable-cli. Each worker thread sets its count only once, when it starts (src/control/jobs.c:509, :554), so the reset count stays in place for the rest of that thread's life.

All three conditions are needed:

• -t below the CPU count
• OMP_NUM_THREADS not set
• lut3d with a compressed LUT

The agent's proposal was above: adding num_threads(dt_get_num_threads()) to DT_OMP_FOR would fix it; a more detailed version:

If the aim is one count that libraries can't override, the fix belongs in darktable's own loops: give every parallel region that uses per-thread buffers an explicit num_threads(dt_get_num_threads()). Then OpenMP never starts more threads than the buffers hold, whatever a library sets, and both the GM reset and the G'MIC risk stop mattering.

• Changing DT_OMP_FOR (darktable.h:131) covers most of these regions.
• 18 bare parallel regions outside the macro would need the same clause. I counted them but didn't check which ones use per-thread buffers.

I rebased the branch onto current master. Claude Code added a new commit to cap the thread count and thus allocate memory according to the effective number of threads. We now have 3 commits: the original crash fix, the cap on the thread count, and the release note. Over to the bot:

The new commit, common: cap the OpenMP thread count at OMP_THREAD_LIMIT, limits darktable.num_openmp_threads to omp_get_thread_limit(). OpenMP never runs more threads than OMP_THREAD_LIMIT, so per-thread buffers sized for --threads held slots that no thread used.

With the cap, OMP_THREAD_LIMIT=N acts like --threads N everywhere darktable reads its thread count. The bilateral grid splits its work into N slices. When N is below 4, darktable also starts fewer background worker threads and does not generate thumbnails in the background. Exports match those made with --threads N, which can differ from the old output by rounding. In 0015-shadhi-bilateral with N = 1, 17 pixels differ by 1/255.

The cap needs #22475. Before that change, X-Trans raw denoise split its work by thread count, so the cap changed its output.

The cap has no separate release note (the issue is about the bug fix in the first commit).

Testing (Linux, 12 CPUs, Release build, CPU path):

• 0068-rawdenoise-xtrans with -t 4, with and without OMP_THREAD_LIMIT=1, is identical to the output of the build without the cap.
• 0015-shadhi-bilateral with -t 4 and OMP_THREAD_LIMIT=1 is identical to -t 1 without the cap. With OMP_THREAD_LIMIT=3, it is identical to -t 3 without the cap.

The cap change has been reviewed by Codex CLI and Antigravity.

@TurboGit : shall I create an issue for the GM >= 3 question? (#22438 (comment))

Created #22567 to track the GMIC 3.x / 4.x issue. Rebased the code to fix the conflict in RELEASE_NOTES.md.